Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[RNMobile][tech-debt] block-list optimisation #22144

Merged
merged 14 commits into from
May 22, 2020
Merged

[RNMobile][tech-debt] block-list optimisation #22144

merged 14 commits into from
May 22, 2020

Conversation

dratwas
Copy link
Contributor

@dratwas dratwas commented May 6, 2020 •
edited
Loading

Description

gutenberg-mobile PR - wordpress-mobile/gutenberg-mobile#2262

In this PR i added some performance improvements in our BlockList and Block components. The big part of these changes is described in my p2 post.

Basically i refactored Block and BlockList to not re-render every time when something changes in the editor.

On the gifs below I added the renders count to each block in the top left corner. It shows how many times the block.native.js render has been called.

Before After
ezgif com-optimize (2) ezgif com-optimize

How has this been tested?

  • the app should work the same as before (maybe a bit faster)
  • the title should be editable
  • inserter point should be rendered in the correct place
  • list footer should work - adds new paragraph on click

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@github-actions
Copy link

github-actions bot commented May 6, 2020 •
edited
Loading

Size Change: 0 B

Total Size: 1.12 MB

ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.62 kB 0 B
build/api-fetch/index.js 3.4 kB 0 B
build/autop/index.js 2.83 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 6.93 kB 0 B
build/block-directory/style-rtl.css 790 B 0 B
build/block-directory/style.css 791 B 0 B
build/block-editor/index.js 105 kB 0 B
build/block-editor/style-rtl.css 10.9 kB 0 B
build/block-editor/style.css 10.9 kB 0 B
build/block-library/editor-rtl.css 7.17 kB 0 B
build/block-library/editor.css 7.17 kB 0 B
build/block-library/index.js 119 kB 0 B
build/block-library/style-rtl.css 7.48 kB 0 B
build/block-library/style.css 7.48 kB 0 B
build/block-library/theme-rtl.css 684 B 0 B
build/block-library/theme.css 686 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 48.1 kB 0 B
build/components/index.js 190 kB 0 B
build/components/style-rtl.css 17.1 kB 0 B
build/components/style.css 17.1 kB 0 B
build/compose/index.js 9.24 kB 0 B
build/core-data/index.js 11.4 kB 0 B
build/data-controls/index.js 1.29 kB 0 B
build/data/index.js 8.42 kB 0 B
build/date/index.js 5.47 kB 0 B
build/deprecated/index.js 771 B 0 B
build/dom-ready/index.js 568 B 0 B
build/dom/index.js 3.11 kB 0 B
build/edit-navigation/index.js 6.63 kB 0 B
build/edit-navigation/style-rtl.css 857 B 0 B
build/edit-navigation/style.css 856 B 0 B
build/edit-post/index.js 302 kB 0 B
build/edit-post/style-rtl.css 12.2 kB 0 B
build/edit-post/style.css 12.2 kB 0 B
build/edit-site/index.js 13.2 kB 0 B
build/edit-site/style-rtl.css 5.45 kB 0 B
build/edit-site/style.css 5.45 kB 0 B
build/edit-widgets/index.js 7.87 kB 0 B
build/edit-widgets/style-rtl.css 4.58 kB 0 B
build/edit-widgets/style.css 4.57 kB 0 B
build/editor/editor-styles-rtl.css 425 B 0 B
build/editor/editor-styles.css 428 B 0 B
build/editor/index.js 44.6 kB 0 B
build/editor/style-rtl.css 5.06 kB 0 B
build/editor/style.css 5.06 kB 0 B
build/element/index.js 4.65 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.64 kB 0 B
build/format-library/style-rtl.css 502 B 0 B
build/format-library/style.css 502 B 0 B
build/hooks/index.js 2.13 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/is-shallow-equal/index.js 711 B 0 B
build/keyboard-shortcuts/index.js 2.51 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.13 kB 0 B
build/list-reusable-blocks/style-rtl.css 226 B 0 B
build/list-reusable-blocks/style.css 226 B 0 B
build/media-utils/index.js 5.29 kB 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.js 3.4 kB 0 B
build/nux/style-rtl.css 616 B 0 B
build/nux/style.css 613 B 0 B
build/plugins/index.js 2.56 kB 0 B
build/primitives/index.js 1.5 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/rich-text/index.js 14.8 kB 0 B
build/server-side-render/index.js 2.68 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.28 kB 0 B
build/url/index.js 4.02 kB 0 B
build/viewport/index.js 1.84 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

@dratwas dratwas marked this pull request as ready for review May 12, 2020 14:07
@dratwas dratwas mentioned this pull request May 14, 2020
Merged
2 tasks
@pinarol pinarol requested review from mchowning and etoledom May 15, 2020 08:51
@dratwas dratwas mentioned this pull request May 18, 2020
Merged
6 tasks
mchowning
mchowning reviewed May 18, 2020
View reviewed changes
Copy link
Contributor

@mchowning mchowning left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking really good and working nicely. Great work @dratwas ! 🎉

I just left a couple of small questions.

packages/block-editor/src/components/block-list/block-list-item.native.js Show resolved Hide resolved
packages/block-editor/src/components/block-list/block-list-item.native.js Show resolved Hide resolved
mchowning
mchowning approved these changes May 19, 2020
View reviewed changes
Copy link
Contributor

@mchowning mchowning left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Nice job @dratwas!

Let's give @etoledom a chance to give the iOS side of this a closer look before we merge.

etoledom
etoledom approved these changes May 22, 2020
View reviewed changes
Copy link
Contributor

@etoledom etoledom left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks very good! Thank you so much for this @dratwas
It was needed for so long 🎉

@dratwas dratwas merged commit a1cf84a into master May 22, 2020
@dratwas dratwas deleted the rnmobile/tech-debt/block-list branch May 22, 2020 14:44
@github-actions github-actions bot added this to the Gutenberg 8.2 milestone May 22, 2020
@mchowning mchowning mentioned this pull request May 29, 2020
Closed
@geriux geriux mentioned this pull request Mar 16, 2023
Merged
@geriux geriux mentioned this pull request Dec 12, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mchowning mchowning mchowning approved these changes

@etoledom etoledom etoledom approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Gutenberg 8.2
Development

Successfully merging this pull request may close these issues.
3 participants
@dratwas @mchowning @etoledom